Skip to content

[RF] Use RooDataHist interfaces with index input arguments#21977

Merged
guitargeek merged 1 commit intoroot-project:masterfrom
guitargeek:issue-6557
Apr 23, 2026
Merged

[RF] Use RooDataHist interfaces with index input arguments#21977
guitargeek merged 1 commit intoroot-project:masterfrom
guitargeek:issue-6557

Conversation

@guitargeek
Copy link
Copy Markdown
Contributor

Use RooDataHist interfaces that take index input arguments, which is safer because they don't rely on the cached last index in the RooDataHist.

Addresses one bullet point in
#6557:

"Correct interface of RooAbsData and derived classes to use e.g. std::size_t for indexing events. int doesn't make sense."

As a followup, we could consider to deprecate the interfaces that use the last cached "current" index.

Use RooDataHist interfaces that take index input arguments, which is
safer because they don't rely on the cached last index in the
RooDataHist.

Addresses one bullet point in
root-project#6557:

"Correct interface of RooAbsData and derived classes to use e.g.
std::size_t for indexing events. int doesn't make sense."

As a followup, we could consider to deprecate the interfaces that use
the last cached "current" index.
@github-actions
Copy link
Copy Markdown

Test Results

    22 files      22 suites   3d 11h 12m 22s ⏱️
 3 836 tests  3 835 ✅  1 💤 0 ❌
76 636 runs  76 618 ✅ 18 💤 0 ❌

Results for commit 51edfcb.

Copy link
Copy Markdown
Member

@lmoneta lmoneta left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!
Thanks for this improvement. Are you going to deprecate the old interfaces (e.g. RooDataHist::weight() ) where you don't pass the index?

@guitargeek
Copy link
Copy Markdown
Contributor Author

Yes, at some point! But I have not decided what to deprecate in 6.42 yet. I always want to deprecate a bit for good reasons per release, but not too much.

@guitargeek guitargeek merged commit 5754e62 into root-project:master Apr 23, 2026
30 of 33 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants